Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

alias command #1815

Closed
wants to merge 18 commits into from
Closed

alias command #1815

wants to merge 18 commits into from

Conversation

void-inject
Copy link

  • Added Alias Mapping
  • Added new Alias Command
  • Modified triggerCommand to check alias mapping

@mwestphal
Copy link
Contributor

@void-inject let me know if you need any help moving forward :)

@void-inject
Copy link
Author

i am OK, will be back to home at 5th january

Copy link

codecov bot commented Jan 8, 2025

Codecov Report

Attention: Patch coverage is 50.00000% with 7 lines in your changes missing coverage. Please review.

Project coverage is 95.61%. Comparing base (7cc980f) to head (7419e07).

Files with missing lines Patch % Lines
library/src/interactor_impl.cxx 50.00% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1815      +/-   ##
==========================================
- Coverage   95.67%   95.61%   -0.07%     
==========================================
  Files         125      125              
  Lines        9950     9963      +13     
==========================================
+ Hits         9520     9526       +6     
- Misses        430      437       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@void-inject
Copy link
Author

i need help with covering lines with test

@mwestphal
Copy link
Contributor

You should be able to add a simple interaction test that input the alias command and use it.

void-inject and others added 3 commits January 8, 2025 22:11
Co-authored-by: Mathieu Westphal <[email protected]>
Co-authored-by: Mathieu Westphal <[email protected]>
Co-authored-by: Mathieu Westphal <[email protected]>
//-------------------------------------------------------------------
private:
// Map to store aliases
std::map<std::string, std::string> aliasMap;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

put that at the end of this class, next to the command map

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@@ -40,6 +40,7 @@ class interactor_impl : public interactor
std::string action, std::function<void(const std::vector<std::string>&)> callback) override;
interactor& removeCommand(const std::string& action) override;
std::vector<std::string> getCommandActions() const override;
std::unordered_map<std::string, std::string> aliasMap;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use the one in the internals instead

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

std::string action = tokens[0];

// Handle Alias Command
if (action == "alias")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ideally alias command should be yet another command add added using addCommand

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think i did it correct

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

return true;
}

// Resolve Alias
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unless Im mistaken this should be done even before tokenizing.

@void-inject
Copy link
Author

is the "check" stuck?

@@ -552,6 +547,11 @@ class interactor_impl::internals
std::map<interaction_bind_t, BindingCommands> Bindings;
std::multimap<std::string, interaction_bind_t> GroupedBinds;
std::vector<std::string> OrderedBindGroups;
//-------------------------------------------------------------------
private:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not needed

//-------------------------------------------------------------------
private:
// Map to store aliases
std::map<std::string, std::string> aliasMap;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keep only this

@void-inject
Copy link
Author

most sad part is when you reliaze how things work after chaos

@void-inject void-inject closed this Jan 8, 2025
@void-inject void-inject deleted the alias_command branch January 8, 2025 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants